fix(v2): validate timeoutSeconds per trigger type (addition) #1877
fix(v2): validate timeoutSeconds per trigger type (addition) #1877IzaakGough wants to merge 25 commits into
Conversation
The v2 SDK accepts any timeoutSeconds value at function-definition
time and leaves the rejection to the Cloud Functions control plane
at deploy time. v1 has had assertRuntimeOptionsValid for a long time
but v2 was missing the equivalent.
Add per-trigger-type validation so misconfigured values fail locally
instead of mid-deploy:
- 540s for event-handling functions (firestore, database, pubsub,
storage, scheduler, eventarc, testLab, remoteConfig, alerts,
dataconnect)
- 3600s for HTTPS and callable functions (onRequest, onCall,
onCallGenkit, identity, dataconnect/graphql, ai)
- 1800s for task queue functions (onTaskDispatched)
Implementation notes:
- New internal helper assertTimeoutSecondsValid(opts, kind) and
MAX_{EVENT,HTTPS,TASK}_TIMEOUT_SECONDS constants in
src/v2/options.ts.
- optionsToEndpoint and optionsToTriggerAnnotations gained an
optional kind argument. When omitted they behave exactly as
before, so existing callers are unaffected.
- Expression<number> and RESET_VALUE are passed through untouched;
the SDK cannot know the concrete value in those cases.
- For providers whose __endpoint is a lazy getter (storage) the
throw happens the first time the manifest is read instead of at
function-definition time; a regression test covers this path.
Fixes #1737
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds validation for timeoutSeconds across all v2 trigger types, ensuring that misconfigured values are caught during function definition rather than at deployment. It introduces specific limits for event-handling (540s), HTTPS/callable (3600s), and task queue (1800s) functions. Reviewer feedback suggests strengthening the validation logic to explicitly handle non-numeric types (like strings or booleans) and clarifying whether a 0-second timeout is valid according to platform documentation.
IzaakGough
changed the title
The v2 SDK accepts any timeoutSeconds value at function-definition
time and leaves the rejection to the Cloud Functions control plane
at deploy time. v1 has had assertRuntimeOptionsValid for a long time
but v2 was missing the equivalent.
Add per-trigger-type validation so misconfigured values fail locally
instead of mid-deploy:
- 540s for event-handling functions (firestore, database, pubsub,
storage, scheduler, eventarc, testLab, remoteConfig, alerts,
dataconnect)
- 3600s for HTTPS and callable functions (onRequest, onCall,
onCallGenkit, identity, dataconnect/graphql, ai)
- 1800s for task queue functions (onTaskDispatched)
Implementation notes:
- New internal helper assertTimeoutSecondsValid(opts, kind) and
MAX_{EVENT,HTTPS,TASK}_TIMEOUT_SECONDS constants in
src/v2/options.ts.
- optionsToEndpoint and optionsToTriggerAnnotations gained an
optional kind argument. When omitted they behave exactly as
before, so existing callers are unaffected.
- Expression<number> and RESET_VALUE are passed through untouched;
the SDK cannot know the concrete value in those cases.
- For providers whose __endpoint is a lazy getter (storage) the
throw happens the first time the manifest is read instead of at
function-definition time; a regression test covers this path.
Fixes #1737
| * HTTPS functions can specify a higher timeout. | ||
| * | ||
| * @remarks | ||
| * The minimum timeout for a 2nd gen function is 1s. The maximum timeout for a |
There was a problem hiding this comment.
I used this PR to test some code review prompts I'm working on. AI on that prompt also called out (same as gemini review bot) that the minimum timeout is 1 not 0.
I noticed you resolved the Gemini review comment. Are you confident the minimum timeout is 0? Why? (I honestly don't know) If it is 0 it'd be good to update this part of the documentation so this file stays consistent with itself.
There was a problem hiding this comment.
I wasn't certain. I worked under the assumption that the minimum timeout for v2 functions was the same as v1 (i.e. 0 seconds).
I double checked and the docs definitely say that the minimum timeout for v1 functions is 0 here
It seems like the underlying cloud run request-timeout docs say that the minimum should be 1 here.
Perhaps we do want a minimum timeout of 1 for v2 then?
There was a problem hiding this comment.
Interesting. Your v1 reference was in Firebase docs so I went digging into that for v2 and found Global Options allows setting timeouts between 0 and 540 seconds: https://firebase.google.com/docs/reference/functions/2nd-gen/node/firebase-functions.globaloptions.md#globaloptions_interface
But I don't see how that works if the cloud run documentation is accurate that it needs to be at least 1s.
Maybe the safest thing to do is try 0 and use that if it works and 1 others. TBH, I'm not sure what behavior I'd expect for 0?
There was a problem hiding this comment.
Hi all, I went digging through the CLI to understand this:
I think 0 means "use the platform default" not a literal zero-second timeout:
1. The CLI treats 0 as "unset" and skips during validation with an if (!timeout) continue
2. The value is sent to the API as a plain protobuf integer - so when the server parses, it's identical to being omitted, so it will be falling back to the default value i think
The doc comment is an inconsistency though, i'd drop the "minimum is 1s"
5 participants
Supercedes #1877
Fixes #1737
Summary
timeoutSecondsvalidation at function-definition or manifest-extraction time using trigger-specific limits: 0-540s for event handlers, 0-3600s for HTTPS/callable functions, and 0-1800s for task queues.null, and invalid non-number types, while preservingExpressionandRESET_VALUEsupport.Testing
npm test -- --grep "assertTimeoutSecondsValid|optionsToEndpoint timeout validation|optionsToTriggerAnnotations timeout validation|v2 provider timeout validation|timeoutSeconds above"npx prettier --check CHANGELOG.md src/v2/options.ts spec/v2/options.spec.ts spec/v2/providers/timeout.spec.ts spec/v2/providers/https.spec.ts spec/v2/providers/storage.spec.ts spec/v2/providers/pubsub.spec.ts spec/v2/providers/tasks.spec.tsnpm run buildNotes
timeoutSeconds: 0remains accepted to match existing runtime option validation behavior.blockingtimeout kind with the platform-specific cap, validate blocking providers before callingoptionsToEndpoint, or keep the current HTTPS kind until the backend/CLI constraint is confirmed.